Skip to content

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented May 15, 2021

This unblocks #84993 in terms of codegen tests, as it brings the MSVC-style (cleanup_pad) EH (LLVM) block order in line with the GNU-style (landing_pad) EH (LLVM) block order, by having both of them be on-demand (instead of MSVC-style being eager and GNU-style lazy/on-demand).

It also unifies the two implementations a bit, similar to #84699, but in the opposite direction (as that attempt made both kinds of EH pads eagerly built).

Opening as draft because I haven't done enough Windows testing just yet, of both this PR, and of #84993 rebased on it. (EDIT: seems to be working as expected)

r? @nagisa

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2021
@eddyb
Copy link
Member Author

eddyb commented May 15, 2021

On MSVC, I was able to bootstrap, and build cargo, ripgrep and syn, with this PR.

Also, rebasing #84993 on this PR did indeed fix the test failure there (i.e. MSVC and GNU styles indeed agree on the LLVM block order).

So I'm un-drafting this PR, it's ready for review I think.

Also, in case anything goes wrong, in the interest of easy bisecting:
@bors rollup=never

@eddyb eddyb marked this pull request as ready for review May 15, 2021 08:29
/// This stores the landing-pad block for a given BB, computed lazily on GNU
/// and eagerly on MSVC.
/// This stores the cached landing/cleanup pad block for a given BB.
// FIXME(eddyb) rename this to `eh_pads`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason these renames aren't happening now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a commit for them if you want, no particular reason, was focusing on getting it working and tested first.

@nagisa
Copy link
Member

nagisa commented May 16, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented May 16, 2021

📌 Commit cb23a79 has been approved by nagisa

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 16, 2021
@bors
Copy link
Collaborator

bors commented May 16, 2021

⌛ Testing commit cb23a79 with merge 747a5d2...

@bors
Copy link
Collaborator

bors commented May 16, 2021

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 747a5d2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 16, 2021
@bors bors merged commit 747a5d2 into rust-lang:master May 16, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 16, 2021
@eddyb eddyb deleted the cg-ssa-on-demand-cleanuppad branch May 16, 2021 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants